Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@cmd/metalprobe/main.go`:
- Around line 40-41: Validate the diskCleaningMode flag immediately after
flag.Parse() in main() by checking that the string value of diskCleaningMode is
one of the allowed values ("quick" or "secure"); if not, print a clear error to
stderr or use log.Fatalf and exit with a non-zero status so the process fails
fast. Locate the diskCleaningMode variable and its usage around flag.Parse() and
replace the current silent acceptance with this explicit validation (and apply
the same validation if diskCleaningMode is set or re-read elsewhere).

In `@internal/controller/server_controller_test.go`:
- Around line 1313-1390: This test creates bmcSecret and server objects but
never deletes them; add explicit cleanup after creation (e.g., immediate defers
or explicit deletions before test exit) to remove the BMCSecret (bmcSecret) and
Server (server) resources via the k8sClient.Delete calls so they don't persist
across tests; locate the creation sites for bmcSecret and server in the It(...)
block and add cleanup logic (defer Expect(k8sClient.Delete(ctx,
bmcSecret)).To(Succeed()) and same for server) or call k8sClient.Delete with the
same objects/refs before returning, ensuring deletion is invoked even on test
failures.

In `@internal/controller/server_controller.go`:
- Around line 423-425: The call to r.handleDiskCleaningCompletion(ctx, server)
currently swallows errors (just logs them), so persistent updates like resetting
CleanOnce/status may never be retried; change the reconcile behavior to
propagate the error instead of ignoring it so the controller requeues the
request (or explicitly requeue with a backoff) when handleDiskCleaningCompletion
fails. Ensure handleDiskCleaningCompletion itself is idempotent (safe to run
multiple times) and performs the persistent update (reset CleanOnce / status
update) in a retryable way (e.g., update the Server via the client with
optimistic retry or patch), and then return any error up to the caller so
reconciler can retry.
- Around line 1303-1311: Change the hard-success wording to indicate an
attempt/completion that may have ignored non-fatal errors: update
server.Status.DiskCleaningStatus.Message from "Disk cleaning completed
successfully" to something like "Disk cleaning attempted during discovery" (or
"Disk cleaning completed; errors may have been ignored"), and when calling
r.Conditions.UpdateSlice for ConditionDiskCleaningCompleted replace the
UpdateReason("DiskCleaningSucceeded") and UpdateMessage("Disk cleaning completed
during discovery") with a non-absolute reason/message such as
UpdateReason("DiskCleaningAttempted") and UpdateMessage("Disk cleaning attempted
during discovery; non-fatal errors may have occurred") so the condition does not
assert an unequivocal success when failures can be ignored.
- Around line 1289-1302: After re-fetching the server object, guard against a
nil server.Spec.DiskCleaningPolicy before dereferencing its DiskCleaningMode:
check if server.Spec != nil and server.Spec.DiskCleaningPolicy != nil and only
then read server.Spec.DiskCleaningPolicy.DiskCleaningMode into
server.Status.DiskCleaningStatus.CleaningMode; otherwise set CleaningMode from
the reconciler default
(metalv1alpha1.DiskCleaningMode(r.DiskCleaningDefaultMode)) so you never
dereference a nil DiskCleaningPolicy when updating
server.Status.DiskCleaningStatus in the Server controller.

In `@internal/probe/diskcleaning_linux.go`:
- Around line 205-220: The code currently writes the completion marker even when
zero disks were actually cleaned; update the logic so
markDiskCleaningCompleted() is only called when at least one disk was
successfully cleaned and there are no failures. Concretely, after computing
successCount, results and skippedCount (symbols: successCount, results,
skippedCount, blockStorage.Disks, markDiskCleaningCompleted), first return an
error if successCount == 0 (e.g., "no disks cleaned" or treat as
skipped/failure), then keep the existing failure check (if successCount <
len(results) return fmt.Errorf(...)); only call markDiskCleaningCompleted() when
successCount > 0 and successCount == len(results).
- Around line 281-303: The code currently uses
exec.CommandContext(...).CombinedOutput() for long-running wiping commands
("shred" and "dd") which can OOM; change both usages to start the command and
stream stdout/stderr instead of buffering: create the *exec.Cmd via
exec.CommandContext (same invocations around shred and dd), call
cmd.StdoutPipe()/cmd.StderrPipe(), start cmd with cmd.Start(), and concurrently
io.Copy from the pipes into a bounded logger sink or io.Discard (or log lines
incrementally) and then wait with cmd.Wait()/cmd.Run() to get the exit status;
keep the existing error-handling logic (fall back from shred to dd and call
rereadPartitionTable on success) but remove CombinedOutput usage so output is
not fully buffered in memory.
- Around line 69-71: The current device validation in validateDevicePath
incorrectly accepts character devices; update the check so it only allows block
devices by verifying fi.Mode() has os.ModeDevice set and does NOT have
os.ModeCharDevice set (i.e., reject when os.ModeCharDevice is present). Modify
the conditional around validateDevicePath's os.ModeDevice check to explicitly
test and return an error if fi.Mode()&os.ModeCharDevice != 0, keeping the
existing error message for non-block devices.

---

Nitpick comments:
In `@api/v1alpha1/server_types.go`:
- Line 118: The DisksProcessed field currently uses the platform-dependent int
type; change its type to a fixed-width integer (e.g., int32) in the struct that
defines DisksProcessed in api/v1alpha1/server_types.go so the CRD schema is
stable across platforms—update the declaration from "DisksProcessed int
`json:\"disksProcessed,omitempty\"`" to "DisksProcessed int32
`json:\"disksProcessed,omitempty\"`" (or int64 if you prefer 64-bit) and run
codegen/CRD generation as needed to propagate the change.